Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

feat: added resolve command #794

Closed
wants to merge 1 commit into from
Closed

feat: added resolve command #794

wants to merge 1 commit into from

Conversation

jbenet
Copy link
Contributor

@jbenet jbenet commented Jun 26, 2018

  • js-ipfs-api is lacking the ipfs resolve command.
  • it is also lacking in the ipfs/specs/core-api docs.
  • but ipfs resolve has been an intended command for a long time

@ghost ghost assigned jbenet Jun 26, 2018
@ghost ghost added the in progress label Jun 26, 2018
@jbenet
Copy link
Contributor Author

jbenet commented Jun 26, 2018

Also, this is bullshit:

  386 passing (17m)
  37 pending
  2 failing

  1) .dht
       "after all" hook:
     Error: Timeout of 80000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.


  2) .log
       .log.level:
     Uncaught AssertionError: expected [Error: socket hang up] to not exist
      at ipfs.log.level (test/log.spec.js:58:26)
      at f (node_modules/once/once.js:25:25)
      at ClientRequest.req.on (src/utils/send-request.js:168:5)
      at Socket.socketOnEnd (_http_client.js:394:9)
      at endReadableNT (_stream_readable.js:974:12)
      at _combinedTickCallback (internal/process/next_tick.js:74:11)
      at process._tickCallback (internal/process/next_tick.js:98:9)



Command failed: mocha --timeout 5000 --ui bdd --colors --exit test/node.js test/**/*.spec.js
null
null
Error: Command failed: mocha --timeout 5000 --ui bdd --colors --exit test/node.js test/**/*.spec.js
null
null
    at Promise.all.then.arr (/Users/jbenet/git/js-ipfs-api/node_modules/execa/index.js:236:11)
    at process._tickCallback (internal/process/next_tick.js:103:7)
pre-push:
pre-push: We've failed to pass the specified git pre-push hooks as the `test`
pre-push: hook returned an exit code (1). If you're feeling adventurous you can
pre-push: skip the git pre-push hooks by adding the following flags to your push:
pre-push:
pre-push:   git push -n (or --no-verify)
pre-push:
pre-push: This is ill-advised since the push is broken.
pre-push:

Tests taking 17min to run for a one line change that i'm absolutely sure has nothing to do with the tests. this kind of friction is a great way to not receive contributions. I almost gave up and didnt push this at all. Use CI, don't waste contributors' time. I really don't buy that this is the way to prevent bad PRs. let CI show the bad PRs

also git push -n didn't work. i had to delete .git/hooks/pre-push. this also wasted time.

@jbenet
Copy link
Contributor Author

jbenet commented Jun 26, 2018

CI is failing due to what appears to be a broken or flaky test, not this PR.


  1) .log .log.level:

     Uncaught AssertionError: expected [Error: socket hang up] to not exist

      at ipfs.log.level (test/log.spec.js:58:26)

      at f (node_modules/once/once.js:25:25)

      at ClientRequest.req.on (src/utils/send-request.js:168:5)

      at Socket.socketOnEnd (_http_client.js:423:9)

      at endReadableNT (_stream_readable.js:1064:12)

      at _combinedTickCallback (internal/process/next_tick.js:138:11)

      at process._tickCallback (internal/process/next_tick.js:180:9)

@jbenet
Copy link
Contributor Author

jbenet commented Jun 26, 2018

BTW, this command (ipfs resolve) does something unique that ipfs name resolve doesn't do. it resolves everything, including ipfs paths like /ipfs/Qmfoo/a/b/c -> /ipfs/Qmbar (which ipfs name resolve does not do).

jbenet referenced this pull request in ipfs-shipyard/ipfs-hubot Jun 26, 2018
@jbenet
Copy link
Contributor Author

jbenet commented Jun 26, 2018

Pls don't purge this commit, as it's being used as a dep for another package: ipfs-shipyard/ipfs-hubot@3a1ce58#r29494914

@alanshaw
Copy link
Contributor

Thanks @jbenet this LGTM. I'll add the command to the spec and also some tests for it and get this merged asap.

FWIW I'm +1 on removing the pre-push hook since we have a stable CI setup, strict peer review process and the lead maintainer protocol.

@daviddias
Copy link
Contributor

daviddias commented Jun 27, 2018

👍 from me on removing pre-push.

As a note, for git push it is not -n, it has to be --no-verify.

Before merging this PR, @alanshaw @jbenet can you make sure it gets documented at http://github.com/ipfs/interface-ipfs-core?

@daviddias
Copy link
Contributor

For pre-push ipfs/aegir#243

@daviddias daviddias changed the title added resolve command feat: added resolve command Jul 4, 2018
@alanshaw alanshaw mentioned this pull request Aug 6, 2018
@alanshaw
Copy link
Contributor

alanshaw commented Aug 6, 2018

Closing this and continuing at #826 so that we can squash and land cleanly without removing commit
3a1ce58.

For the time being, DO NOT DELETE THIS BRANCH 😃

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants